fix(config): keep wheelsVersion enforcement working on stamped release builds - #3189
Conversation
…e builds PackageLoader.$normalizeWheelsVersion() guarded against unstamped dev builds with a literal `local.raw == "@build.version@"` comparison. The release pipeline (tools/build/scripts/prepare-core.sh) does a GLOBAL line-oriented `sed s/@build.version@/<version>/g` over every .cfc in the artifact, so on a shipped 4.0.3 build that guard became `local.raw == "4.0.3"` — the real runtime version normalised to "0.0.0" and $isCompatibleVersion() took the dev-build skip branch for every package. Net effect: wheelsVersion constraints were silently unenforced on every released build (4.0.0-4.0.3). Detect the placeholder by its structural shape (prefix `@build.` + suffix `@`) instead, mirroring BuildInfo.cfc::isDev(), which is structural for exactly this reason — the fragments are not full placeholder tokens, so the sed pass leaves them untouched. Add a build-time sanity check in prepare-core.sh that fails the build if a stamped self-version sentinel reappears in PackageLoader.cfc, plus a source-scan regression spec mirroring buildInfoSpec.cfc. Refs #3178 Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Wheels Bot — No doc updatesReviewed this PR's diff and found no docs that need updating (purely internal bug fix that restores already-documented behavior: the user-facing |
![wheels-bot[bot]](https://avatars.githubusercontent.com/in/3657588?s=60&v=4){kind=small}
There was a problem hiding this comment.
Wheels Bot — Reviewer
TL;DR: This PR fixes #3178 — PackageLoader.$normalizeWheelsVersion()'s literal "@build.version@" guard was rewritten by prepare-core.sh's global sed into local.raw == "<release version>" on every shipped artifact, silently disabling wheelsVersion enforcement for all packages on all released builds. The swap to a structural prefix/suffix check is correct, faithfully mirrors the existing BuildInfo.cfc::isDev() pattern, and ships layered regression guards (build-time grep + source-scan spec). Verdict: comment — no blocking findings, one minor test-coverage nit.
Correctness
Verified clean — no findings:
- The structural check (
vendor/wheels/PackageLoader.cfc:1380-1383) is sound:Len(local.raw) >= 8 && Left(local.raw, 7) == "@build." && Right(local.raw, 1) == "@". The real call site (Global.cfc:3444→application.wheels.version←BuildInfo.version(),events/onapplicationstart.cfc:105) passes"0.0.0-dev"on dev checkouts and the concrete stamped version on releases, so a stamped"4.0.3"now correctly normalizes to itself and$isCompatibleVersion()enforces constraints. The placeholder branch covers embedding callers that feed the raw placeholder directly (as the spec does). - It is an exact mirror of
BuildInfo.cfc:57(return left(v, 7) == "@build." && right(v, 1) == "@";), which is structural for precisely this reason — the claimed prior art is real. PackageLoader.cfcnow contains zero@build.version@literals (verified withgrep -cat the head SHA), so the global sed atprepare-core.sh:59has nothing to clobber.- The build guard (
tools/build/scripts/prepare-core.sh:158-166) runs after all sed passes and greps for the exact stamped shapelocal.raw == "${VERSION}". It only catches that one reintroduction shape, but the source-scan spec backstops it by failing on any literal occurrence anywhere in the file — sensible defense in depth.
Cross-engine
No findings. Left() is called with a constant 7 (never 0, so the Lucee 7 Left(str, 0) crash — Cross-Engine Invariant 8 — cannot fire), and SpanExcluding/Right/Len are safe on all CI engines. The spec uses arrow functions and FileRead(ExpandPath(...)) consistent with buildInfoSpec.cfc:57.
Tests
- Nit (non-blocking): the two behavioral tests in
PackageLoaderVersionStampSpec.cfc(lines 49–66 and 70–83) nearly duplicate existing coverage inPackageLoaderSpec.cfc— "rejects packages whose wheelsVersion constraint the runtime cannot satisfy" (lines 271–292,wheelsVersion = "4.0.0") and "treats dev build stamp as permissive…" (lines 321–332, identical fixture + assertions). The genuinely new, TDD-failing coverage is the source-scan test, which the PR body states honestly. Keeping the behavioral pair colocated as self-documenting regression context for #3178 is defensible, so this is informational only — but if you'd rather avoid drift between the duplicate pairs, the two behavioralit()blocks could be dropped or replaced with a cross-reference comment.
For the record, I considered flagging the literal wheelsVersion = "@build.version@" at PackageLoaderVersionStampSpec.cfc:75 (specs ship in the artifact via prepare-core.sh:28 and get sed-stamped, inverting that test's input on a shipped build) — but this is the established repo idiom (PackageLoaderSpec.cfc:324, buildInfoSpec.cfc:25), and core specs assert source-tree state throughout, so it's not a defect introduced by this PR.
Docs
- Changelog fragment present and well-formed:
changelog.d/3178-packageloader-version-stamp-guard.fixed.md(fixedtype, complete bullet, references #3178). No directCHANGELOG.mdedit. ✓ - Guide/AI-doc updates deferred to
bot-update-docs.ymlper the checklist — consistent with the pipeline convention.
Commits
Single commit 90cac06a3 conforms to commitlint.config.js: type fix, header 78 chars, body lines ≤ 100, not ALL-CAPS, DCO Signed-off-by present. The message explains the why (sed clobbering, silent enforcement bypass) rather than restating the diff. ✓
|
Superseded by #3194 (merged): a propose-fix draft that raced the ForgeBox path-repair campaign for the same issue; the campaign PR landed first and closed the underlying issue. Closing the duplicate. |
1 participant
Summary
PackageLoader.$normalizeWheelsVersion()guarded against unstamped dev builds with a literallocal.raw == "@build.version@"comparison. The release pipeline (tools/build/scripts/prepare-core.sh) does a global, line-orientedsed s/@build.version@/<version>/gover every.cfcin the artifact, so on a shipped 4.0.3 build that guard becamelocal.raw == "4.0.3"— the real runtime version normalised to0.0.0,$isCompatibleVersion()took the "unstamped dev build → skip enforcement" branch, andwheelsVersionconstraints went silently unenforced for every package on every released build (4.0.0–4.0.3, 4.0.3 verified by extracting the published artifact).This swaps the literal comparison for a structural placeholder check (prefix
@build.+ suffix@), mirroringBuildInfo.cfc::isDev()— which is structural for exactly this reason. The prefix/suffix fragments are not full placeholder tokens, so the globalsedpass leaves them untouched. A build-time sanity check is added toprepare-core.sh(alongside the existing BuildInfocommitSubjectcheck) that fails the build if a stamped self-version sentinel ever reappears inPackageLoader.cfc, and a source-scan regression spec mirrorstests/specs/buildInfoSpec.cfc.Related Issue
Fixes #3178
Type of Change
Feature Completeness Checklist
Signed-off-by:vendor/wheels/tests/specs/packages/PackageLoaderVersionStampSpec.cfc(source-scan regression guard + behavioural coverage of stamped-release vs placeholder-shaped runtimes)bot-update-docs.ymlbot-update-docs.ymlbot-update-docs.ymlchangelog.d/3178-packageloader-version-stamp-guard.fixed.mdwheelsCLI (which embeds the Lucee runtime) is not installed here, sobash tools/test-local.shcannot start its server. The genuinely-failing assertion in the new spec is a pure source scan, which I verified deterministically with the spec's own logic:vendor/wheels/PackageLoader.cfccontained 2@build.version@literals before the fix (toBe(0)fails) and 0 after (toBe(0)passes). CI runs the full suite across every engine × DB.Test Plan
PackageLoaderVersionStampSpec.cfcassertsPackageLoader.cfccontains zero@build.version@sentinels. Before the fix: 2 occurrences (comment + guard) → fails. After: 0 → passes. Mirrors the regression guard inbuildInfoSpec.cfc."4.0.3") rejects theincompatversionfixture (>=99.0) and loadscompatversion(>=3.0); a loader constructed with a placeholder-shaped version stays permissive and loads both.prepare-core.shnow fails the build if the post-sedPackageLoader.cfccontainslocal.raw == "<version>".Run locally (with the Wheels CLI installed):